fix: resolve relative/absolute path mismatch in ts_build_dep_graph + add Ruby test coverage module#598
Conversation
…d add Ruby test coverage module The ts_build_dep_graph function normalized resolved import paths to absolute but checked membership against file_set which contains relative paths (as returned by find_source_files). This caused all import edges to be silently discarded. Fix: check the resolved path against file_set first as-is, then try the alternate form (absolute <-> relative) before discarding. This works regardless of whether file_set uses relative paths (most backends via find_source_files) or absolute paths (some test fixtures and backends). Also adds a Ruby-specific test_coverage.py module with: - parse_test_import_specs: extracts require/require_relative paths - resolve_import_spec: resolves relative paths from test files - has_testable_logic: detects def/class/module definitions - strip_test_markers: strips _spec.rb, test_*.rb, *_test.rb suffixes - strip_comments: strips # comments preserving string literals Without this module, Ruby's dep graph correctly builds edges but _parse_test_imports returns an empty set, leaving all scripts marked as untested.
|
Thanks @wbsmolen. I accepted the underlying TypeScript graph problem but did not apply this PR as-is. Decision: rejected in favor of the root #604 fix. The graph portion overlapped with #572/#604 and is now fixed locally in commit a9289015 with explicit source-root vs tsconfig-root handling. I did not apply the Ruby coverage portion because the Stage 1 conditions remain unresolved: it needs regression tests and broader Validation passed for the graph fix:
Closing this PR because the accepted graph bug is fixed locally and the Ruby portion needs a follow-up with the requested tests/behavior. |
|
Closing after fixing the accepted TypeScript graph issue locally in commit a9289015. The Ruby coverage part still needs the Stage 1 conditions before landing. Thanks again for the contribution. |
2 participants
Problem
ts_build_dep_graphsilently discards all import edges whenfile_setuses relative paths.Root cause:
find_source_filesreturns relative paths, sofile_set = set(file_list)contains entries likescripts/asc_client.rb. But the function then normalizesresolvedto an absolute path viaos.path.normpath(os.path.join(scan_path, resolved)), making the membership checkif resolved not in file_setalways True — so no edge is ever added.This affects all language plugins that use tree-sitter dep graph building (TypeScript, PHP, Ruby, etc.) whenever
find_source_filesis used as the file finder.Reproduction: Point desloppify at a Ruby project with
require_relativeimports. The dep graph will always be empty{}despite correct tree-sitter parsing.Fix
Try the resolved path as-is first, then try the alternative form (absolute ↔ relative) before discarding. This handles both:
file_set+ relativeresolved(most backends viafind_source_files)file_set+ relativeresolved(existing test fixtures)Ruby test coverage module
Also adds
desloppify/languages/ruby/test_coverage.pywith the required hooks:parse_test_import_specs: extractsrequire/require_relativepathsresolve_import_spec: resolves relative paths from spec files to production fileshas_testable_logic: detectsdef/class/moduledefinitionsstrip_test_markers: strips_spec.rb,test_*.rb,*_test.rbsuffixesstrip_comments: strips#comments while preserving string literalsWithout this module,
get_lang_hook("ruby", "test_coverage")returnedNone, leaving all Ruby scripts marked as untested regardless of the dep graph state.And wires it in via
generic_lang(..., test_coverage_module=_test_coverage_mod)inruby/__init__.py.Tests
5543 passed, 153 skipped (1 pre-existing failure in
test_bundled_syncunrelated to this change).